Add ElasticsearchIndexDeletionPolicy and EngineConfig policy wrapper#130442
Merged
fcofdez merged 15 commits intoelastic:mainfrom Jul 28, 2025
Merged
Add ElasticsearchIndexDeletionPolicy and EngineConfig policy wrapper#130442fcofdez merged 15 commits intoelastic:mainfrom
fcofdez merged 15 commits intoelastic:mainfrom
Conversation
tlrx
added
>non-issue
:Distributed/Engine
elasticsearchmachine
added
the
serverless-linked
kingherc
reviewed
Jul 2, 2025
Contributor
kingherc
left a comment
kingherc
left a comment
There was a problem hiding this comment.
Awesome work. Some early comments.
server/src/main/java/org/elasticsearch/index/engine/ElasticsearchIndexDeletionPolicy.java
Outdated
Show resolved
Hide resolved
fcofdez
reviewed
Jul 16, 2025
|
|
||
| public abstract boolean hasUnreferencedCommits(); | ||
|
|
||
| public interface CommitsListener { |
Contributor
There was a problem hiding this comment.
I was thinking about getting rid of this interface since we don't use it anymore, wdyt @kingherc? we can do in a follow-up once all the pieces are in place.
Contributor
There was a problem hiding this comment.
Hi @fcofdez . Yes I believe it can be removed.
Separately, we were discussing whether IndexModule#setIndexCommitListener() could be removed in this thread with @tlrx , feel free to read to see as well. I had a question whether someone, e.g., externally, like a plugin writer user, might have been using it, but we think no. So that may also be considered for removal. However, maybe an optional follow-up.
Contributor
There was a problem hiding this comment.
Yep, that was my impression too. I'll remove it in a follow-up so we keep this PRs manageable.
elasticsearchmachine
added
the
Team:Distributed Indexing (obsolete)
Collaborator
|
Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing) |
kingherc
approved these changes
Jul 18, 2025
|
|
||
| public abstract boolean hasUnreferencedCommits(); | ||
|
|
||
| public interface CommitsListener { |
Contributor
There was a problem hiding this comment.
Hi @fcofdez . Yes I believe it can be removed.
Separately, we were discussing whether IndexModule#setIndexCommitListener() could be removed in this thread with @tlrx , feel free to read to see as well. I had a question whether someone, e.g., externally, like a plugin writer user, might have been using it, but we think no. So that may also be considered for removal. However, maybe an optional follow-up.
server/src/main/java/org/elasticsearch/index/engine/CombinedDeletionPolicy.java
Outdated
Show resolved
Hide resolved
szybia
added a commit
to szybia/elasticsearch
that referenced
this pull request
Jul 28, 2025
…-tracking * upstream/main: Fix MergeWithLowDiskSpaceIT testRelocationWhileForceMerging (elastic#131806) [ML] Prevent the trained model deployment memory estimation from double-counting allocations. (elastic#131990) ES|QL Assert current thread during query planning and execution (elastic#131807) Add ElasticsearchIndexDeletionPolicy and EngineConfig policy wrapper (elastic#130442) [TEST] Adds tests for ESTestCase randomSubset methods (elastic#131745) Simplify esql session (elastic#131925) Simplify EsqlExecution info serialization (elastic#131823) Add utility to check for project global block (elastic#131927) [DOCS] Update ES|QL applies to's (elastic#131805) Handle structured log messages (elastic#131027) Mute org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT test {p0=search/600_flattened_ignore_above/flattened ignore_above multi-value field} elastic#131967 Mute org.elasticsearch.xpack.remotecluster.CrossClusterEsqlRCS2EnrichUnavailableRemotesIT testEsqlEnrichWithSkipUnavailable elastic#131965 Mute org.elasticsearch.xpack.restart.FullClusterRestartIT testWatcherWithApiKey {cluster=UPGRADED} elastic#131964 [ES|QL] Fix aggregate_metric_double sorting and mv_expand issues (elastic#131658) Reduce logging levels for meter usage tests (elastic#131935)
afoucret
pushed a commit
to afoucret/elasticsearch
that referenced
this pull request
Jul 28, 2025
…lastic#130442) This change adds a new abstract class ElasticsearchIndexDeletionPolicy that allows using different policy implementations for InternalEngine. It also adds an index deletion policy wrapper to the EngineConfig to allow wrapping the default CombinedDeletionPolicy by inheritors of InternalEngine. Relates ES-10929 --------- Co-authored-by: Francisco Fernández Castaño <francisco.fernandez.castano@gmail.com>
breskeby
pushed a commit
to breskeby/elasticsearch
that referenced
this pull request
Feb 11, 2026
Draft pull request for a possible solution for tracking the Lucene commits acquired/released while the underlying engine instances are reset. This solution is based on `LocalCommitsRefs` which tracks the acquired commit generation at the shard level. This instance is then passed to the Lucene index deletion policies used by the IndexEngine and HollowIndexEngine. So far it only take care of commits acquired using `Engine#acquireIndexCommit(boolean)` for snapshots. Local reader tracking is not supported (yet, I hope to be able to reuse `LocalCommitsRefs` there too). If we move forward with this solution, I'll split this draft into multiple PRs. Requires elastic#130442 Relates
breskeby
pushed a commit
to breskeby/elasticsearch
that referenced
this pull request
Feb 11, 2026
Draft pull request for a possible solution for tracking the Lucene commits acquired/released while the underlying engine instances are reset. This solution is based on `LocalCommitsRefs` which tracks the acquired commit generation at the shard level. This instance is then passed to the Lucene index deletion policies used by the IndexEngine and HollowIndexEngine. So far it only take care of commits acquired using `Engine#acquireIndexCommit(boolean)` for snapshots. Local reader tracking is not supported (yet, I hope to be able to reuse `LocalCommitsRefs` there too). If we move forward with this solution, I'll split this draft into multiple PRs. Requires elastic#130442 Relates
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds a new abstract class ElasticsearchIndexDeletionPolicy that allows using different policy implementations for InternalEngine.
It also adds an index deletion policy wrapper to the EngineConfig to allow wrapping the default CombinedDeletionPolicy by inheritors of InternalEngine.
Relates ES-10929